bpo-44897: WIP: Integrate trashcan into _Py_Dealloc#27738
Closed
nascheme wants to merge 5 commits intopython:mainfrom
Closed
bpo-44897: WIP: Integrate trashcan into _Py_Dealloc#27738nascheme wants to merge 5 commits intopython:mainfrom
nascheme wants to merge 5 commits intopython:mainfrom
Conversation
Loading
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a WIP/proof-of-concept of doing away with Py_TRASHCAN_BEGIN and
Py_TRASHCAN_END and instead integrating the functionality into
_Py_Dealloc. There are a few advantages:
all container objects have the risk of overflowing the C stack if a
long reference chain of them is created and then deallocated. So, to
be safe, the tp_dealloc methods for those objects should be protected
from overflowing the stack.
the Py_TRASHCAN_BEGIN and Py_TRASHCAN_END macros are hard to
understand and a bit hard to use correctly. Making the mechanism
internal avoids confusion. The code can be slightly simplified as
well.
This proof-of-concept seems to pass tests but it will need some careful
review. The exact rules related to calling GC Track/Untrack are subtle
and this changes things a bit. I.e. tp_dealloc is called with GC
objects already untracked. For 3rd party extensions, they are calling
PyObject_GC_UnTrack() and so I believe they should still work.
The fact that PyObject_CallFinalizerFromDealloc() wants GC objects to
definitely be tracked is a bit of a mystery to me (there is an assert to
check that). I changed the code to track objects if they are not
already tracked but I'm not sure that's correct.
There could be a performance hit, due to the _PyType_IS_GC() test that
was added to the _Py_Dealloc() function. For non-GC objects, that's
going to be a new branch and I'm worried it might hurt a bit. OTOH,
maybe it's just in the noise. Profiling will need to be done.
https://bugs.python.org/issue44897